Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update typeReferenceBlock definition #18933

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

leandrommedeiros
Copy link
Contributor

Description (*)

Including the "class" attribute to the typeReferenceBlock definition allows you to update a the class attribute of a block tag when overriding or updating a layout XML file without the need to remove the original block and re-add it with the same name but different class and without using a preference in the Dependency Injector.

Manual testing scenarios (*)

Simple scenario:

  1. On your custom module, create a layout file
  2. Add the tag update with the handle to the any other layout you wish to use as basis.
  3. Add a referenceBlock tag to any block found in the original layout, using the same name as the original but with a new value for the class attribute.
  4. Create your own block class in the Vendor/Module/Block directory and voilá, you are now able to have your own module, with a layout based on any other module, with a new block behavior and, since you didn't use any preference, the original module still works the same.

Scenario I applied:

My wish was to create my own Admin Report using the Magento's Report New Accounts as a basis, but without the "Period" dropdown in the toolbar, so here's what I did:

  1. On my module MyVendor_CustomReports I've created the file view/adminhtml/layout/report_customer_vip_grid.xml with the following content:
<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <update handle="reports_report_grid"/>
    <body>
        <referenceBlock name="adminhtml.report.grid" class="MyVendor\CustomReports\Block\Adminhtml\Grid">
            ...
        </referenceBlock>
    </body>
</page>
  1. I created the new Block class MyVendor\CustomReports\Block\Adminhtml\Grid extending the \Magento\Reports\Block\Adminhtml\Grid class.

  2. I overrided the value of the attribute $_template to MyVendor_CustomReports::grid.phtml.

  3. I copied Magento/Reports/view/templates/grid.phtml to MyVendor/CustomReports/view/templates/grid.phtml and edited it to erase the select element.

This worked as fine as any other fallback, but it was only possible editing the elements.xsd file (this PR), otherwise you get a "invalid attribute 'class'" exception.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

Including the "class" attribute to the typeReferenceBlock definition allows you to update a block class when overriding or updating a layout.xml file without the need to remove the original block and re-add it with the same name but different class.
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 29, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @leandrommedeiros. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@orlangur orlangur self-assigned this Oct 29, 2018
@orlangur
Copy link
Contributor

@leandrommedeiros nice catch! This is exactly what we needed in scope of #11709 (comment) already.

@antonkril @buskamuza could you please clarify if there was some reasoning behind disallowing class change via referenceBlock?

@buskamuza
Copy link
Contributor

I don't remember a specific reason.
One thing I can think about is that it's possible to specify a class from a different hierarchy, so any other customization (a template) may be broken because new block doesn't provide the same set of methods. This can be covered with a test.
It would be really helpful if you could submit a proposal to https://github.com/magento/architecture to make sure we don't miss anything with this change.

@sivaschenko
Copy link
Member

Hi @leandrommedeiros could you please submit a proposal as a PR to https://github.com/magento/architecture to be approved by architects. cc @orlangur

@orlangur
Copy link
Contributor

@sivaschenko I'm supposed to do it actually including discussion already performed in #app-design Slack channel.

@sidolov
Copy link
Contributor

sidolov commented Nov 28, 2018

Hi @leandrommedeiros , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

@sivaschenko
Copy link
Member

Reopening the PR as it is a dependency for #11709 . A proposal was submitted to Architectural Discussions that is scheduled 23rd January 2019: magento/architecture#59 (comment)

@sivaschenko
Copy link
Member

@orlangur @leandrommedeiros please consider attending the public architecture meetup 23rd January to discuss this change.

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buskamuza @sivaschenko I'm not sure what's the process is but from magento/architecture#85 this architectural change seems to be approved.

@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-4563 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@leandrommedeiros thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@soleksii soleksii self-assigned this Apr 2, 2019
@soleksii
Copy link

soleksii commented Apr 2, 2019

✔️ QA Passed

Before:

before

After:

after

@m2-assistant
Copy link

m2-assistant bot commented Apr 8, 2019

Hi @leandrommedeiros, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants